Fixes for Dandi file access #1477
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes three issues related to NWB file access through Dandi:
- Fix #1473: Refactored
_open_nwb_fileto handle raw NWB file path resolution internally, ensuring files are cached using the correct key - Fix #1472: Removed redundant code that forced
nwb2load_filepathinto fetch attributes, avoiding unnecessary external table access - Fix #1428: Added
str()conversion to ensureAnalysisNwbfile.get_abs_pathconsistently returns string objects when fetching from schema
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/spyglass/utils/nwb_helper_fn.py | Centralized Dandi path resolution logic in _open_nwb_file to properly handle both direct file paths and raw paths, and simplified get_nwb_file by consolidating path checking conditions |
| src/spyglass/utils/mixins/analysis.py | Added explicit string conversion for path returned from schema externals table to ensure consistent string return type |
| src/spyglass/utils/dj_helper_fn.py | Removed redundant code that unnecessarily added nwb2load_filepath to fetch attributes, as this attribute is populated in the loop regardless |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes Error accessing dandi-loaded raw files #1473
__open_nwb_filesusing the copy filenameFixes Error for fetching non-local files #1472
externalstable when fetching nwb filesFixes Error in fetch_nwb due to inconsistent path type #1428
AnalysisNwbfile.get_abs_pathreturn a string objectChecklist:
CITATION.cffaltersnippet for release notes.CHANGELOG.mdwith PR number and description.